-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Refactor PgVercorStore filter template to use JSONB field access #589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thank you @dperezcabrera, looks like a nice improvement! Could you please pull rebase to the upstream main (e.g. |
Changed the filter template in PgVercorStore to replace JSONPath expressions with JSONB field access, using standard SQL operators such as '=' instead of '==', 'AND' instead of '&&', and 'OR' instead of '||'. This adjustment addresses compatibility issues with the 'IN' operator, which previously returned parse errors. Also updated PgVectorFilterExpressionConverter and corresponding tests to align with these changes. - Replaced `metadata::jsonb @@ '$.key == "value"'::jsonpath` with `metadata::jsonb->>'$.key' = 'value'` - Fixed parsing issues with `metadata::jsonb @@ '$.key in ["value"]'::jsonpath` by using JSONB field access. - Updated logical operators IN filter expressions to standard SQL syntax. These changes improve the clarity, execution reliability, and compatibility of filter expressions in the database.
Thank you. I've rebased and merged the changes as requested! |
@dperezcabrera have you run the integration tests for the pgvector and for the autoconfiguraiton? |
I have executed them; however, since I don't have an API key for OpenAI, this has been the result: Can you please send me the error logs by email? |
@dperezcabrera for example
|
I see another problem, |
Hi, this is a great contribution to improve the speed of pgvector, you mention I can't easily share access keys, I would recommend getting an openai subscription, the cost is minimal. |
I need to know the type of the key. If fieldname is an integer, the method must use this template (metadata->>'fieldname')::INTEGER. If fieldname is a string, then: metadata::jsonb->>'fieldname'. If fieldname is a date, (metadata->>'fieldname')::DATE. This ensures the key is correctly interpreted in the database query. Now I have and access key, before I only had API keys for Microsoft's Azure, and they didn't work with the integrated tests. |
bumping to RC1, apologies |
I think it is best to close this pull request, and address this functionality in the future. |
Thank you for the effort @dperezcabrera. We can re-open it in the future if you have time to work on it. Causing due to some unsolved issues (check the log above). |
Changed the filter template in PgVercorStore to replace JSONPath expressions with JSONB field access, using standard SQL operators such as '=' instead of '==', 'AND' instead of '&&', and 'OR' instead of '||'. This adjustment addresses compatibility issues with the 'IN' operator, which previously returned parse errors. Also updated PgVectorFilterExpressionConverter and corresponding tests to align with these changes.
metadata::jsonb @@ '$.key == "value"'::jsonpath
withmetadata::jsonb->>'key' = 'value'
metadata::jsonb @@ '$.key in ["value"]'::jsonpath
by using JSONB field access.These changes improve the clarity, execution reliability, and compatibility of filter expressions in the database.